feat(cloud-agent-next): enforce container billing - #5358
Conversation
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| ? await this.getPendingStopReason(context.generation) | ||
| : undefined; | ||
| await super.destroy(); | ||
| if (context) await updateBillingContext(this.ctx.storage, context); |
There was a problem hiding this comment.
WARNING: destroy() restores a pre-super.destroy() billing snapshot and can undo concurrent settlement.
This restore is not serialized with the heartbeat controller lock. While super.destroy() is awaited, a heartbeat can advance usageMeasuredAtMs, after which writing the captured context back rewinds already-billed seconds and double-counts them on the later physical recordStop. Re-read storage after destroy and restore only keys that are still missing, or skip the write when a newer measurement or settlement is already present.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| try { | ||
| // The control plane may fail after the deadline. Keep the durable block and | ||
| // reissue destroy until the physical stop hook settles the generation. | ||
| await this.destroy(); |
There was a problem hiding this comment.
WARNING: Successful destroy() is never reissued, so a no-throw control-plane miss never settles.
The comment says to keep issuing destroy until onStop settles the generation, but the retry is only in the catch path. scheduleForceStop already deleted other billingForceStop alarms, and this restore does not re-arm a heartbeat. If destroy() returns and onStop is dropped or delayed, the generation stays open with no 5s retry and no 120s killer. Re-arm billingForceStop until the generation is gone or onStop has closed it.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const metadata = await this.getMetadata(); | ||
| if (!metadata || !isCloudAgentContainerBillingEnabled(this.env, metadata.identity)) | ||
| return false; | ||
| return createAgentSandbox(this.env, metadata).isBillingBlocked(); |
There was a problem hiding this comment.
WARNING: Reconnect billing checks fail open when the sandbox RPC errors.
isContainerBillingBlocked() already established that enforcement is enabled, then calls isBillingBlocked() with no enforcement bit. isSandboxBillingBlocked() therefore returns false on a throw, so ingest reconnect accepts the socket and in-flight work continues. The facade path in this PR passes enforcementRequested and then requires admission. Treat a failed probe as blocked, or call ensureBillingAdmission() here too.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Executive SummaryForce-destroy restore can rewind billed time or leave a paid generation unclosed, and reconnect billing checks fail open when the sandbox RPC errors. Overview
Issue Details (click to expand)WARNING
Files Reviewed (6 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 77f42f7)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 77f42f7)Status: 3 Issues Found | Recommendation: Address before merge Executive SummaryForce-destroy restore can rewind billed time or leave a paid generation unclosed, and reconnect billing checks fail open when the sandbox RPC errors. Overview
Issue Details (click to expand)WARNING
Files Reviewed (42 files)
Reviewed by grok-4.6 · Input: 87.9K · Output: 21K · Cached: 521.7K Review guidance: REVIEW.md from base branch |
1 participant
Summary
Rollout
recordStartV2before enabling any Cloud Agent payer flagsCONTAINER_BILLING_SERVICESin this PRVerification
pnpm testandpnpm typecheckinpackages/container-usagepnpm test,pnpm typecheck, andpnpm lintinservices/container-usage-meterpnpm test:postgresinservices/container-usage-meterpnpm test,pnpm typecheck,pnpm lint, andpnpm test:wrapperinservices/cloud-agent-nextpnpm test,pnpm typecheck, andpnpm lintinservices/session-ingestgit diff --cached --checkbefore commit